-
Notifications
You must be signed in to change notification settings - Fork 920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade to Docusaurus 3 #1335
base: staging
Are you sure you want to change the base?
Upgrade to Docusaurus 3 #1335
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fill-the-fill - this is very welcome and I would be inclined to merge it rather than remain on the obsolescent version of Docusaurus (FYI for group: this builds v3.5.2
).
It builds a working site locally that I can't find any UI/content problems with, but the diagnostic warnings generated by the yarn build
are huge... enough to overwhelm any messages about legitimate problems or things to clean up. This includes reported broken links and missing anchors on pages that appear to be working correctly on the built site.
The main issue was with pulling markdown files such as CIPS and others. Some of the files didn’t follow the correct formatting, which caused breaks on our side.
I don't know what to make of this, since I can personally guarantee the CIP editors have been rigorous about following Markdown formatting conventions, and have been fixing linking & other formatting issues that caused things to break on the Dev Portal for the 2+ years I've been involved in both projects. Any real breach of Markdown rules would have been identified on the previous version of Docusaurus.
It seems likely that CIP formatting problems would be in how the files are translated after downloading from GitHub... e.g. there are references to these "broken anchors" on many CIPs, for anchors that don't exist in the source material:
- Broken anchor on source page path = /docs/governance/cardano-improvement-proposals/CIP-0014:
-> linking to CIP-0001#cip-format-and-structure (resolved as: /docs/governance/cardano-improvement-proposals/CIP-0001#cip-format-and-structure)
-> linking to CIP-0001#cip-workflow (resolved as: /docs/governance/cardano-improvement-proposals/CIP-0001#cip-workflow)
Likewise there are reported problems on the documentation pages, in some cases broken links to autogenerated content like the section headings in the article sidebar.... though these were working normally for the pages I checked.
So I can approve this if this is the best we can get for now, but I do think it would first be worth a look at what generates this huge trail of debugging output & solve those cases one by one before merging this. If any source material on the CIPs repository is an issue then let's identify such problems here and I'll fix them upstream immediately.
Regarding that, I've ran It's not really the problem with how it's documented, it's how Docusaurus wants it to be :D |
I don't get why you cut out all the truncate comments in the blog posts, we need them for the preview. Also the md to mdx seems to be not necessary, what's the reasoning behind that? Thanks. |
This PR contains too many problems, assumptions and decisions that will cause us problems in the near future. In this state it should be converted in a draft. |
I've stated above the reasoning behind converting certain files from .md to .mdx, you are more than welcome to test out those files in .md format and see it for yourself. blog blog |
I've added back truncates and fixed most of the warnings. At the moment the only warnings are CIP files related. |
@fill-the-fill it's true: the broken rendering of the CIP content from GitHub still generates a lot of spurious warnings. Are you suggesting that the Markdown content of the CIPs themselves is the true reason for this? If so, can you give an example in the actual CIP content? |
For example, in
Example: Those warnings won't break the website, its mostly just redirecting to a specific section on the same CIP page. In our case it just won't jump to that section when being clicked. Besides that, all other warnings should be cleared out now. You can take at the latest commit I've managed to remove most of the warnings this morning by adjusting CIP and CPS scripts, but here inside of build is the list of warnings that should probably be taken care of on CIP repo instead |
thanks @fill-the-fill - I'd be ready to post an issue on the CIPs repo to decide on a standard for anchor links that will be most portable outside of GitHub, including Docusaurus. But first I need to understand exactly what this version of Docusaurus does not "like" about these links. I thought it might be a matter of capitalisation (with all lowercase being canonical), but while that explains the first warning (CIP-0001) it's not an issue with the second (CIP-0002 first item): since the anchor Docusaurus "likes" is also used everywhere in the CIP-0002 source. And at the end you see Docusaurus selecting "canonical" links that have initial or camel capitalisation, so it can't be as simple as that (see here, where GitHub generates a lowercase link while Docusaurus says it wants initial capitalisation). So unless you can explain the rules that links in the source documents are expected to follow, we may need more verbose information about how the warnings are being flagged... we'd need a line number and/or other precise context, not just the file that link is in. (Could you set an option in the build script that's more verbose about where the warning is?) Once we can put those rules into an issue on the CIPs repository then we might start making these corrections preemptively. Authors might not be able to write their documents that way, but we've talked about adding a GitHub CI for the CIPs repo that will do some format massage & file updates when checking in pull requests & we could add something there when that is set up. |
@rphair thanks for your reply. It seems like the issue with the example in CIP-2 is that some of the links are broken and can't be read, look at the example bellow: Here is another issue, those should be redirecting to a specific part of the page, but they're not recognized, in the example bellow you can see I'm clicking on the link, usually it goes to that part of the page, in this example nothing happens: |
In my opinion, this is not needed, but I could be wrong, for example:
Easily can be found by searching Besides that, it shouldn't be a blocker to proceed with this PR, imho |
There are aren't duplicate warnings, but the links which generate the warnings are duplicated in individual CIP documents. See |
I will come back to you very soon |
|
thanks @fill-the-fill - that's very helpful. I do think eventually we should have diagnostics from |
@rphair sounds good, let me know if there is anything else I can do |
Ok looks like build broke again, I will check it soon |
Thanks for the effort again @fill-the-fill and everyone that is participating here. What I'm concerned about is: the list of compromises is getting longer and longer to maintain this. We are at a point where it is questionable not to remove the CIP from the portal. cips.cardano.org has been greatly improved in the last few months, you could try to index that so that you can still search for CIP content with the Developer Portal search but then land directly on cips.cardano.org. |
@katomm that's a compelling suggestion, but I would want to bring in the last known devs of cips.cardano.org (@ptrdsh @aldo555, cc @KtorZ) because the content there has been out of sync with the CIP GitHub material for long periods: and is currently out of sync again (as seen in this last enormous & high visibility update to CIP-0001):
as well as this very high visibility addition to the CIP archive, merged yesterday, still not being synced: Last time, as reported in the PR, the issue was an "expired token" but since less than 1 month had gone by when we observed this was out of date I don't think it could be a token problem again (?). So I would be OK with coupling the Dev Portal to the CIPs web site if both:
|
@rphair We will very soon be updating cips.cardano.org to be automatically rebuilt once a push is made to the cips repo. |
no @katomm that's a welcome comment & not really off topic because it was the basis for just referring the CIP other editors again back here in a parallel thread. Here's the other one of the 2 with the same potential breakage (at least in GitHub's YAML parser, a backtick is only a problem in the initial position: but its use will be imitated by the community if we allow it anywhere):
I'll push through a PR to fix these upstream right away & have asked the others for opinions about #1335 (comment). |
#1335 (comment) fixed in cardano-foundation/CIPs#930. |
Builds should no longer be failing for this reason, as of cardano-foundation/CIPs@e4c5f48. |
Checklist
yarn build
after adding my changes without getting any errors.yarn.lock
(or have removed these changes).Updating documentation or Bugfix
Hey 😎
I finally got some time to play around and update Docusaurus 3+.
I've made quite a few changes, and everything seems to be working now, including the previously broken Changelog.
The main issue was with pulling markdown files such as CIPS and others. Some of the files didn’t follow the correct formatting, which caused breaks on our side. Creating a special case for each file would have been a nightmare since every file that was pulled had its own missing comma or bracket that was really hard to track.
Since the biggest issue with upgrading to Docusaurus 3.0 was the transition to MDX, one of the biggest game-changers that helped was Experimental Commonmark. That quickly took care of the issue.
I added the following code to
docusaurus.config.js
:However, this ended up breaking some of our markdown files, specifically those using the
<Tabs>
element. The solution I came up with was to transition those files from.md
format to.mdx
. I've decided to leave the decision of transitioning all the files to.mdx
up to everyone else.Please feel free to test it out locally and check if I've missed anything, thank you!